Be more lenient with trim() #32
Conversation
Allowing for null and undefined values to return empty string. I'm not sure if you would rather deal with this in the function calling trim() but this will at least allow some leeway when dealing with attributes that may be null or contain a string.
|
Mmmm. I'm inclined to reject this because I want to keep the behavior of trim as close to String.prototype.trim as possible. You can't do |
|
You're right of course, I should have spent more time explaining this. The issue has to do with null values. The DatetimeFormatter does not appear to handle null values correctly. Because trim() is called on all the raw values, an exception is raised. An example of a failing test: it(".fromRaw() ignores null values", function () {
var formatter = new Backgrid.NumberFormatter();
expect(formatter.fromRaw(null)).toBe(null);
});Or a higher level test, for DateCell: it("handles a null value", function () {
var cell = new Backgrid.DateCell({
model: new Backbone.Model({
date: null
}),
column: {
name: "date",
cell: "date"
}
});
cell.render();
expect(cell.$el.hasClass("date-cell")).toBe(true);
});My patch avoids the problem by adding a special case to trim in order to avoid the exception being raised TypeError: String.prototype.trim called on null or undefined There's probably a better place to handle it, checking for null / undefined before going to trim(). Or name the function differently :) |
|
You want |
|
No -- I want nothing displayed in those cells. Right now it throws an exception. How else would you indicate the absence of a value? |
|
Ok I need you to do a couple of things. First, rebase your branch against 0.2.0-wip, I like your test cases, but I think the fix should be inside the See what you can do about this line. https://github.com/wyuenho/backgrid/blob/0.2.0-wip/src/formatter.js#L193 When you are done them I'll merge. Thanks. |
|
Hi @bruno-c, are you still working on this? |
|
@wyuenho I would love to take care of this immediately, but I am currently a bit swamped with day-to-day work. I'll try to find some time later on today, if that works for you. |
|
Yep no problem. I'm trying to release a maintenance release of 0.1.2 that's why I was asking. Thanks a bunch in advance! |
…atetimeFormatter
|
Closing this pull request - the new one is #43 |
#32 Handle null, undefined and incorrect values in DatetimeFormatter
Allowing for null and undefined values passed to trim() to return empty string.
I'm not sure if you would rather deal with this in the functions calling trim() but this will at least allow some flexibility handling attributes that may be null or contain a string.